release: stage audited 7.14.3 Bitcoin-only fork product - #627
Open
BitHighlander wants to merge 90 commits into
Open
BitHighlander wants to merge 90 commits into
BitHighlander wants to merge 90 commits into
Conversation
Co-authored-by: BitHighlander <4935895+BitHighlander@users.noreply.github.com>
…eanup fix(signing): reject empty message sessions
…diation fix(security): consolidate 7.14.2 audit remediation
There was a problem hiding this comment.
🔵 Needs a closer look
It changes multiple security-critical subsystems (RNG, seed ceremony, storage/versioning, signing/address semantics, and CI/release provenance) across many files, warranting final human review despite strong test coverage.
Pull request overview
This PR advances the 7.14.x release line to v7.14.3 and implements/locks down the “Bitcoin-only + Dice entropy” release delta while hardening seed-time RNG handling, Taproot/BIP-340/341/86 support, storage compatibility boundaries, and CI/release gating.
Changes:
- Add Bitcoin-only build variant (compile-time stripping of non-Bitcoin engines, message map entries, coin/token tables, and CI/release matrix coverage for both variants).
- Add/extend Bitcoin security features: Taproot address/script support, OP_RETURN disclosure hardening, multisig quorum validation, tx input digest reset helper, and new regression/unit tests.
- Add seed-time RNG health gate plus Dice entropy reset ceremony (with debug-link observability and extensive unit tests), plus CI hardening (gitleaks pin+scoping, aggregate “ci-gate”, nonzero propagation).
File summaries
| File | Description |
|---|---|
| unittests/firmware/transaction.cpp | New unit tests for taproot input weight, OP_RETURN/Omni disclosure paging, multisig quorum rejection. |
| unittests/firmware/test_board.cpp | One-time board bootstrap guard for unit test binaries to prevent hangs. |
| unittests/firmware/signing.cpp | New unit tests for change-path/script-type constraints, checksum encoding, multisig quorum validation, abort scrubbing. |
| unittests/firmware/setup_ceremony.cpp | Tests for abort wiping mnemonic + recovery fragments; recovery finalize disarm behavior. |
| unittests/firmware/rng_health.cpp | New unit tests for RNG health analysis, chunking equivalence, and fail-closed semantics. |
| unittests/firmware/fsm.cpp | Conditionally exclude non-Bitcoin FSM abort workflow tests from bitcoin-only builds. |
| unittests/firmware/dice.cpp | New tests for dice roll counts and deterministic mix vectors. |
| unittests/firmware/confirm_test_utils.cpp | New confirm auto-accept driver via emulator tiny UDP messages (ButtonAck + DebugLinkDecision). |
| unittests/firmware/CMakeLists.txt | Build manifest refactor + bitcoin-only conditional compilation of altcoin suites. |
| unittests/firmware/authenticator.cpp | New authenticator cache/authorization-boundary tests with confirm driver integration. |
| unittests/crypto/CMakeLists.txt | Add BIP340/BIP341 crypto test compilation. |
| unittests/crypto/bip340.cpp | Add verbatim BIP-340 vectors + BIP-86 address vectors + BIP-341 key-path witness checks. |
| scripts/verify-token-def.py | New generator guard: fail build if generated token defs have zero usable rows. |
| scripts/emulator/firmware-unit.sh | Preserve make xunit exit status and propagate it after copying reports. |
| scripts/emulator/docker-compose.bitcoin-only.yml | New compose overlay to build/run emulator+unit container with KK_BITCOIN_ONLY=ON. |
| scripts/emulator/capture-dice-flow.py | New evidence tool to drive/reset dice flow and capture OLED screenshots. |
| lib/rand/rng.c | Latch STM32 RNG seed/clock errors in software + emulator test seams + strong random_buffer for some linkers. |
| lib/rand/rng_health.c | New RNG source liveness check + SP800-90B RCT/APT + latched verdict + checked draw/continuous observe. |
| lib/rand/CMakeLists.txt | Add rng_health.c to kkrand library sources. |
| lib/firmware/u2f.c | Route U2F key-handle derivation path through checked RNG; fail registration on RNG self-test failure. |
| lib/firmware/txin_check.c | Add txin_dgst_reset_current() helper to preserve last-tx duplicate warning while resetting current digest. |
| lib/firmware/transaction.c | Taproot address encoding, scriptPubKey derivation helper, OP_RETURN confirm behavior for bitcoin-only, multisig quorum validation, taproot input weight. |
| lib/firmware/ton.c | Adjust Ed25519 signing call sites to updated signature (no explicit public-key pointer). |
| lib/firmware/storage.h | Add SUS_BitcoinOnlyLocked status for refusing incompatible storage without wiping. |
| lib/firmware/storage.c | Bitcoin-only storage band stamping/locking, RNG-gated key-material draws, stronger session-clear scrubbing, refuse commit when locked. |
| lib/firmware/solana.c | Adjust Ed25519 signing call sites to updated signature (no explicit public-key pointer). |
| lib/firmware/reset.c | Add dice-entropy reset flow, roll digest lifecycle, RNG health failure reporting, and broader abort scrubbing. |
| lib/firmware/recovery_cipher.c | Hoist/rename scratch buffers to make abort wiping reliable; add debug test seams for fragment wiping. |
| lib/firmware/messagemap.def | Exclude non-Bitcoin message handlers and outputs in bitcoin-only builds. |
| lib/firmware/fsm.c | Centralize derived-node scrub, bitcoin-only no-op abort hooks, and bitcoin-only “locked wallet” refusal in init flows. |
| lib/firmware/fsm_msg_solana.h | Adjust Ed25519 signing call site to updated signature. |
| lib/firmware/fsm_msg_debug.h | Expose dice digest via DebugLinkState. |
| lib/firmware/fsm_msg_common.h | Advertise taproot capability; set stable bitcoin-only variant strings; bound GetCoinTable to build variant; tighten wipe/session boundary. |
| lib/firmware/fsm_msg_coin.h | Enforce BIP86 (m/86’) path/script-type invariants for Taproot address requests. |
| lib/firmware/dice_input.c | New on-device dice entry UI/flow with debounce/hold classification and debug-link injection support. |
| lib/firmware/coins.c | Exclude tokens from coin table in bitcoin-only builds; add BIP86 path mismatch checks. |
| lib/firmware/CMakeLists.txt | Conditional compilation/linking of non-Bitcoin engines; add dice input; add token-table verification step. |
| lib/firmware/authenticator.c | Add cache wipe/test seams; harden OTP generation cleanup/zeroization paths. |
| lib/firmware/app_layout.c | Fix multi-line bech32 address rendering so displayed address doesn’t silently truncate. |
| lib/board/keepkey_flash.c | Gate OTP randomness-block programming on checked RNG draw; fail closed without locking bad entropy. |
| include/keepkey/transport/messages.options | Add dice_digest sizing; bound DebugLinkDecision.input to keep tiny-message decode within 64B; size clearsign fields. |
| include/keepkey/transport/messages-thorchain.options | Add ThorchainMsgSend.denom sizing. |
| include/keepkey/transport/messages-solana.options | Add Solana sizing entries for schema/LUT/signature fields and other buffers/counts. |
| include/keepkey/transport/messages-ripple.options | Add RippleSignTx.memo size limit. |
| include/keepkey/transport/messages-ethereum.options | Add LoadClearsignSigner size limits. |
| include/keepkey/rand/rng.h | Add seed/clock error latch API + emulator test seams; include stdbool. |
| include/keepkey/rand/rng_health.h | New public RNG health/checked-draw API and explicitly scoped coverage list. |
| include/keepkey/firmware/txin_check.h | Export txin_dgst_reset_current(). |
| include/keepkey/firmware/transaction.h | Export multisig quorum predicate + input scriptPubKey helper. |
| include/keepkey/firmware/storage.h | Define shipped version floor + bitcoin-only version band + locked-wallet query API. |
| include/keepkey/firmware/signing.h | Expose pure helpers for unit tests + debug seams for state wipe checks. |
| include/keepkey/firmware/reset.h | Extend reset_init signature to include dice_entropy; add dice digest accessor. |
| include/keepkey/firmware/recovery_cipher.h | Add debug test seams for recovery fragment wiping verification. |
| include/keepkey/firmware/fsm.h | Export derived-node scrub + debug test seams. |
| include/keepkey/firmware/dice_input.h | New dice-entropy API surface (roll collection + mix). |
| include/keepkey/firmware/coins.h | Exclude token indices from bitcoin-only builds. |
| include/keepkey/firmware/coins.def | Exclude non-Bitcoin coin definitions from bitcoin-only builds. |
| include/keepkey/firmware/authenticator.h | Add cache/test seam declarations and include stdbool. |
| docs/security/7.14.3-bitcoin-only-dice-audit-sop.md | New audit SOP defining line-review ledger, gates, and evidence expectations. |
| docs/DiceEntropy.md | New user-facing doc describing dice entropy flow, guarantees, and limitations. |
| deps/crypto/CMakeLists.txt | Add bip340.c to trezor-crypto build. |
| CMakeLists.txt | Bump version to 7.14.3; add KK_BITCOIN_ONLY option; ensure BITCOIN_ONLY define is always 0/1; disable unused coin crypto in bitcoin-only build. |
| .github/workflows/release.yml | Release matrix for full + bitcoin-only artifacts; per-variant hash manifests with provenance. |
| .github/workflows/ci.yml | Least-privilege permissions; pinned/scoped gitleaks; build/test matrix for both variants; compose overlays; per-variant artifacts; global ci-gate job. |
Review details
- Files reviewed: 68/68 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
BitHighlander
force-pushed
the
release/7.14.3-bitcoin-only
branch
from
August 27, 2026 20:02
6024f60 to
0f7903d
Compare
Owner
Author
Audit index — 7.14.3 Bitcoin-onlyThis is the durable top-level ledger. Detailed checkboxes live in the commit-pinned line maps below. A checkbox means every listed base and candidate range in that file was reviewed; findings must cite the exact range and head. Immutable review identity
Canonical dependency identity
The direct firmware DP pin intentionally remains Line-addressable audit surfaces
Gates
Head-change log
Copilot delivery status
No merge, tag, signing, publishing, release, upstream firmware PR, or physical-test claim is made by this fork audit vehicle. |
Squash the audited 7.14.3 delta onto the immutable 7.14.2 review base: Bitcoin-only product boundaries, Taproot signing, Dice entropy and RNG health, storage compatibility, disclosure and signing fixes, dependency pins, tests, and release evidence gates.
BitHighlander
force-pushed
the
release/7.14.3-bitcoin-only
branch
from
August 28, 2026 18:54
c636edf to
abe29d1
Compare
Owner
Author
7.14.3 line audit — firmware deltaExact range: Checkbox rule: mark a file complete only after reviewing every listed base and candidate range, including removed/replaced base lines. Findings must cite the surface, path, range, and exact head. Generated files and tests are reviewable evidence, not substitutes for reviewing runtime code. Changed files represented in this checklist: 117. 7143-BTC — Bitcoin, Taproot, signing and exact disclosure
7143-CI — CI, release provenance, artifacts and failure propagation
7143-DEP — dependency identity and build integration
7143-DOC — security contract and operator documentation
7143-ENT — Dice, RNG, setup, storage and secret lifecycle
7143-PROD — Bitcoin-only product boundary and link surface
7143-REG — cross-chain and regression validation
7143-REG — cross-chain, parser, UI and session hardening
|
Owner
Author
7.14.3 line audit — trezor-crypto dependencyExact range: Checkbox rule: mark a file complete only after reviewing every listed base and candidate range, including removed/replaced base lines. Findings must cite the surface, path, range, and exact head. Generated files and tests are reviewable evidence, not substitutes for reviewing runtime code. Changed files represented in this checklist: 57. DEP-CRYPTO-BTC — secp256k1, Schnorr and Taproot primitives
DEP-CRYPTO-CORE — shared hashes, encodings and build surface
DEP-CRYPTO-ED — Ed25519 API and callers
DEP-CRYPTO-TEST — dependency tests and vectors
DEP-CRYPTO-ZEC — Pallas, RedPallas and Orchard primitives
|
Owner
Author
7.14.3 line audit — device-protocol dependencyExact range: Checkbox rule: mark a file complete only after reviewing every listed base and candidate range, including removed/replaced base lines. Findings must cite the surface, path, range, and exact head. Generated files and tests are reviewable evidence, not substitutes for reviewing runtime code. Changed files represented in this checklist: 21. DEP-DP-BOUNDS — nanopb capacity and allocation bounds
DEP-DP-GEN — generators, documentation and repository wiring
DEP-DP-SCHEMA — protobuf message and wire-ID definitions
|
Owner
Author
7.14.3 line audit — Python companion dependencyExact range: Checkbox rule: mark a file complete only after reviewing every listed base and candidate range, including removed/replaced base lines. Findings must cite the surface, path, range, and exact head. Generated files and tests are reviewable evidence, not substitutes for reviewing runtime code. Changed files represented in this checklist: 81. DEP-PYK-CI — companion CI, catalogs and generation gates
DEP-PYK-DOC — packaging, fixtures and documentation
DEP-PYK-LIB — host API, protobuf bindings and transaction logic
DEP-PYK-TEST — device integration, vectors and report coverage
|
BitHighlander
changed the base branch from
review/7.14.2-pr458-current
to
develop
September 8, 2026 19:59
Picks up the reset-ceremony fix: the Internal Entropy screen is branched on the actual per-product policy instead of a pre-7.15 version floor. On 7.14.2 the old predicate expected a ButtonRequest that the product deliberately does not send, which failed mid-ceremony and left the emulator in PIN entry, cascading into the next test's wipe_device. Canonical keepkey:reconcile/upstream-sync @ 45e19bc582cd4eba2c0703eec76ac52e5564f7e1.
Floors the taproot must-run entries at 7.14.3, the release taproot ships in, instead of 7.0.0. Applied to 7.14.2 the old floor demanded six taproot cases from a product with no supports_taproot and no P2TR path, which validate-junit failed as skipped-but-required. The requirement stays binding on 7.14.3 and 7.15, which both carry taproot. Canonical keepkey:reconcile/upstream-sync @ 8649abeaeb8f60543e162bb3ebcd8bf25243d70c.
7.14.3 was the only product still honouring ResetDevice.display_random. 7.14.2 ignores it outright and 7.15 removed the parameter, so the newest of the three re-opened a disclosure both its neighbours close. Fork issue #542. The screen rendered all 32 bytes of the device RNG draw and was drawn strictly BEFORE EntropyRequest, so it disclosed the exact half whose complement the host itself supplies: anyone reading the OLED who also knows ext_entropy computes SHA256(shown || ext_entropy), the mnemonic pre-image. Revealing before the counterparty commits is a commitment scheme run backwards. Two in-tree claims were wrong and are corrected rather than copied: - "the displayed value is already post-mix" — false on every reachable path, since display_random and dice_entropy were mutually exclusive, so dice_mix() never ran when the screen was drawn. The value shown was the raw RNG draw. - the implied Trezor lineage. Trezor's legacy firmware had byte-for-byte this same ordering and removed the feature in PR #4119 (core 2.8.3, legacy 1.13.0), calling it purposeless and a false sense of security. Nothing shipped requests it: hdwallet answers EntropyRequest from crypto.getRandomValues inside the transport and never surfaces the value, so the audit the screen was supposed to enable could not be completed on our own host stack even in principle. The field stays in the wire schema and is ignored, as on 7.15 — it is not reserved, because keepkeyctl still passes it positionally. Removes both guards that existed only to constrain the screen (the _no_backup refusal and the dice mutual exclusion), which also repairs scripts/emulator/capture-dice-flow.py: it requested display_random and dice_entropy together, so the guard answered it with a SyntaxError and the script could not run against its own firmware. reset.c, reset.h and the dice block are now identical to 7.15 apart from 7.15's BIP-85 mnemonic scratch. dice_input.c was already byte-identical. docs/DiceEntropy.md gains a ColdCard comparison and states the remaining gap plainly: a user can prove their rolls were captured, but not that they reached the seed, because seed = SHA256(SHA256(int_entropy || rolls) || ext_entropy) holds two values the user does not have.
display_random is now ignored on every product, so the reset tests assert the device does not draw an internal-entropy screen instead of branching on version. Canonical keepkey:reconcile/upstream-sync @ 7f538a95f00fdea59879ceab1a684762fe408411.
Side-by-side of the two dice models, written to answer one question: can a user prove the device used their rolls? Ours says no, and the reason is the derivation, not the dice-entry UI, which is comparable. KeepKey: seed = SHA256(SHA256(int_entropy || rolls) || ext_entropy) ColdCard: seed = SHA256(rolls) (Dice-Rolls-Only) Three inputs against one. The user holds exactly one of ours, and ext_entropy is generated inside the hdwallet transport and never surfaced, so it is not recoverable even by a cooperating user. Records why ColdCard can safely display its own pre-mix TRNG entropy while our equivalent screen could not: their protocol has no entropy command, so the complement of anything shown is dice the user never transmits. The rule is about the complement, not about showing. Also records the July 2026 ColdCard RNG disclosure, where a five-year PRNG regression cut the effective search space to about 72 bits and the one mitigating factor was users who had rolled dice. That is the argument for a dice-only mode: mixing protects you when your dice are bad, dice-only protects you when the device is bad, and the second is the failure that actually happened to a shipping vendor. Proposes dice_only = 11 on ResetDevice, an offline verifier, the full 32-byte digest, roll bias rejection, and that dice-only never be the default. Fixes a stray double blank line left in reset.c by the display_random removal.
…everywhere Builds no longer wait on static analysis: `needs` is the three fast gates (lint-format, check-submodules, secret-scan) and cppcheck runs beside them, multi-threaded, with its build dir cached per commit (prefix restore -- the build dir re-checks each file's own hash, so a stale entry is safe). The base image is pulled once, with retries, and loaded from the cache in every job that needs it; the integration job builds the compose services once and runs the two containers without --build. The exit codes are initialised before `|| RC=$?` (which assigns only on failure), and the clang-format install retries because apt.llvm.org intermittently serves an empty key and lint-format now gates the whole graph. Measured on the fork: 10.1 min -> 7.4 min wall for 7.14.3.
Every one is a regression this release introduced, or a claim it makes that
the code does not keep. Each has a test that fails without the fix -- run
against a control build with the fixes reverted, not just against the fix.
Auto-lock, which this release extended to AWAY_FROM_HOME:
- idle_time is never reset by host traffic, so it measured wall-clock since
the device left the home screen rather than idleness. A cipher recovery or
a large signing stream that outlasted the delay was torn down mid-flight
("Not in Recovery mode" after 24 words of entry). Host frames now count as
activity, but only AWAY_FROM_HOME, so polling a device sitting at the home
screen still cannot hold it unlocked.
- the lock path aborts signing, and signing_abort() draws the home screen,
which reset the timer it had just fired on. The screensaver was replaced by
the home screen one tick later, on a device that had just locked. The timer
is preserved across the abort.
PIN handling:
- pin_protect() checks every entered PIN against the wipe code, and any PIN
that is not the wipe code returns PIN_WRONG, which cleared the session --
which this release made abort all workflows. A dry-run recovery stages its
ceremony before prompting, so on a PIN-protected device it disarmed itself
and the first CharacterAck failed. PIN revocation now revokes signing
state; the paths that must end a ceremony (session_clear, the auto-lock,
Initialize, ClearSession) still call fsm_abort_workflows() themselves.
Amounts and rendering:
- the XRP ceiling was 1e11 drops (100,000 XRP), 1e6 below XRP's own maximum,
so payments 7.14.1 signed correctly were refused with a SyntaxError. The
encoding has 62 usable bits; 1e17 drops fits with room to spare.
- MsgDeposit carries "MAYA.CACAO", not "cacao", so every CACAO deposit
rendered at zero decimals: 1 CACAO showed as "10000000000 MAYA.CACAO"
while the same base units showed as "1 cacao" on the MsgSend screen.
- the seed-backup pager measured mnemonic rows into a stack buffer it never
scrubbed, leaving up to 351 bytes of backup words behind.
Claims the code did not keep:
- storage_commit() CRCs sizeof(flash_temp)/4 WORDS, and 2570 bytes is 642
words = 2568 bytes, leaving the last byte of the V17 record outside the
write verification -- while a unit test and a comment asserted the opposite.
The buffer is 2572 bytes (643 words) with static assertions, as 7.15 has.
- layout_debuglink_watermark() was deleted while tools/bootloader still calls
it under DEBUG_LINK, so no debug device image could be built. The
definition is restored, still uncalled by layout_clear().
Verified in the CI container: firmware-unit 204/204, board-unit 19/19,
crypto-unit 18/18.
…sg.value Two defects in the THORChain EVM deposit path, both already fixed on the 7.15 line and missing here. thor_isThorchainTx() matched on the 4-byte selector alone, so calldata shaped like depositWithExpiry() to ANY contract on ANY chain got the THORChain clear-sign screens -- and with them the exemption from the AdvancedMode gate that blocks every other unrecognized contract call. A default device would narrate "Routing through <attacker hex>", "Using Asgard vault <attacker hex>", "Confirm sending 1 ETH" and sign. The pin is (chain_id, address) together, because a router address is an identity on one chain only: Ethereum d37bbe57.. (the current v4.1.1 router; the constant still named the retired v3 address, so the "Thorchain router" label was already dead) and Avalanche 00dc6100.., the two the test suite exercises. A chain with no pinned router falls to the blind-sign gate rather than inheriting the trusted UX. For a native-asset deposit the router forwards msg.value and ignores the ABI amount word, but the amount screen rendered the ABI word. A host could show "Confirm sending 0.01 ETH" on the decoder's own screen while attaching 5 ETH, which only appeared later on the generic fee screen. Native deposits now display msg.value, and a token deposit with value attached is refused -- that value would be swept with no screen representing it. Ethereum.ThorchainDepositIsPinnedToItsRouterOnItsChain covers both routers, the wrong-address, wrong-chain, unpinned-chain and no-chain_id cases. Verified against a control build with the pin removed, where it fails. Full suite in the CI container: 205/205.
recovery_cipher_finalize() gated the commit on words_entered, which counts
SEPARATORS, but built the phrase with strtok(), which collapses runs of them.
A ceremony driven with nothing but spaces therefore satisfied the 12/18/24
count gate while emitting zero words: the auto-complete loop never ran (so its
gate stayed true), enforce_wordlist is false when the host omits it (the wire
default), which skips mnemonic_check() in the commit condition, and
setup_commit() stored the EMPTY mnemonic as the device seed and answered
"Device recovered". That seed is PBKDF2("", "mnemonic") -- public. Any funds
sent to the "recovered" wallet are spendable by whoever sent the spaces.
The same shape accepts a one-word phrase. Count the words the loop actually
emitted and require that to equal the count the ceremony claimed, on every
path including the dry run.
Recovery.SpacesOnlyCeremonyIsRefusedAndCommitsNothing drives the ceremony
end to end -- init, twelve space CharacterAcks, finalize -- and asserts the
device is still uninitialized. Against a control build without this fix it
fails with storage_isInitialized() == true, i.e. the seed was committed.
Full suite in the CI container: 206/206.
The deposit and send screens each decided how to scale an amount. 7.15's send screen drifted from the formatter's rule and scaled every host-chosen denom by CACAO's 1e10, showing an amount the signed document did not contain; the same two-copies shape exists here. Extract mayachain_decimalsForDenom() so every MAYAChain screen asks the same function. No behaviour change on this line: the formatter already applied 10 decimals to CACAO and 0 to everything else, and the test pins that.
Pinning the deposit decoder to THORChain's routers refused Maya Protocol deposits, which use the same calldata shape through their own mainnet router (e3985e6b..) -- the pinned integration suite calls that address "firmware- pinned" and its two MAYAChain EVM tests went red. Both protocols are narrated by these screens, so both routers belong in the pin, and the screen now names which one it matched instead of hard-coding "Thorchain router". The chain scope is unchanged: the Maya router is an Ethereum-mainnet identity, so the same 20 bytes on another chain do not match.
- a bitcoin-only-locked device refused settings writes only inside storage_commit(), which returns void: ChangePin, ChangeWipeCode, ApplySettings and ApplyPolicies took the user through their confirmations and reported success while nothing was written. Guard the four handlers up front, the way 7.15 already does, and leave the commit-side check as the backstop it was meant to be. (ChangeWipeCode's body is behind an ifdef that is off today; the guard is inert until it is turned on.) - the transport failure handler aborted every workflow in production, which discards a setup ceremony a transport hiccup has nothing to do with. It ends signing, as it was meant to. - "Maximum priority fee" could be lower than what the runtime charges: the implicit compute-unit limit assumed 200,000 units for every instruction and ignored the 3,000-unit builtin allocation (SIMD-0170), so the screen was not an upper bound. 7.15 has the same defect and gets the same fix separately. - the boot RNG gate was never entered by its own tests: they exercised a seam that latches regardless, so removing the gate would not have failed them. Verified in the CI container, BOTH variants: firmware-unit 210/210 full, 107/107 bitcoin-only, board-unit 19/19.
…others 7.14.2 and 7.15 refuse to link when the gap between the end of static allocation and the stack falls below 16 KiB -- the gate that exists because RC7 shipped an 11.2 KB gap under a 12.4 KB msg_write() frame, which overwrites static memory on the boot path. This line carried the .confidential size assert but not the runtime one, so the same regression could ship here and only surface as a boot fault on hardware.
… pointer The ARM build refuses `char* conf = <string literal>` under -Werror=discarded-qualifiers; the emulator's clang build does not, so the router-label change compiled clean locally and broke build-arm-firmware. Same class as the note in the tree: an emulator build proves nothing about ARM.
The recorded pins and validated head predated the audit, and the receipt named a device-protocol commit that cannot build the shipped dice ceremony. Replace them with the head, tree, pins and CI run that were actually verified, state the host results from that run, and record what the audit covered and fixed on this line. Documentation only: the validated head named in the receipt is this commit's parent, and this commit changes no code, test or pin.
The dice audit SOP stated invariants the shipped two-mode ceremony deliberately does not hold: the modes are opt-in and host-selected, and DICE ONLY drops device entropy by design so the seed can be recomputed offline from the rolls alone. A document that contradicts the firmware is worse than none, because the next auditor reads it as the specification and files the difference as a defect -- or, worse, "corrects" the firmware to match it. The bitcoin-only release asset also shipped a reproduction command that builds the other product's binary, so a reader following it gets a hash mismatch and concludes the release is unreproducible.
Names the head, tree and CI run this candidate was validated at, itemises the skips rather than summing them, and records every audit finding on this line with its resolution in audit-units/. Nothing but documentation changes here, so the validated head named inside is this commit's parent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
`node` is the shared fsm_derived_node scratch, and it is scrubbed only by the next derivation or by fsm_abort_workflows(). Neither runs on the two error exits after the derivation: a compile error is answered with ethereum_signing_abort(), which scrubs ethereum.c's own privkey and nothing else. So a host that sends an ERC-20 transfer whose encoded recipient does not match the derived to_address_n leaves a private key sitting in a static buffer, on demand. 7.14.2 already fixed this; the hunk did not travel. This is that hunk, comment included, so the three lines read the same. No seam: fsm_derived_node is static in fsm.c and the transfer path has no native driver. The commit message says so rather than shipping a test that would assert nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
sig_with_hashtype[73] holds a signature of up to 73 bytes -- every size nanopb can decode for MultisigRedeemScriptType.signatures -- and then appends the sighash byte at [sig_len], which at 73 is one past the end. The comment above it says this "removes the out-of-bounds write itself rather than relying on that cap", and it did not: signing_validate_input()'s 72-byte cap was still the only thing in the way. Sizing the scratch from the field makes the claim true and cannot drift if the cap ever moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
ethereumFormatAmount() takes the Wanchain tx type from a module static that ethereum_signing_init() owns. On the transfer path the amount screen is drawn BEFORE signing_init() runs, so a Wanchain transaction left its type behind and the next transfer's amount screen named the asset " WAN" on whatever chain it was really on -- a wrong ticker beside a right number, which is the shape of mistake a confirmation screen exists to prevent. The formatter now sets that type from the message it was handed. signing_init() assigns the same value again later, so nothing downstream changes. Ethereum.TransferTickerComesFromThisMessageNotTheLastOne pins it, with the Wanchain leg as an in-test control so a build that never set the ticker could not pass the Ethereum assertion for the wrong reason. The test asserts the ticker is derived from the message passed in; reproducing the stale-static sequence itself needs the fsm path, which has no native driver. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
…before rng_health_check() runs before random_buffer() and proves only that the source was sound when the draw started. A seed or clock fault that latches while these very bytes are being produced was noticed on the NEXT call, after this one had already been handed to the caller -- and this path exists precisely to feed key material. rng_health_gate() re-reads SEIS/CEIS on every sampling iteration for exactly this reason. This applies the same rule on the consumer side, and wipes the buffer the way the continuous-test refusal beside it already does. No seam: the test hooks latch the mirror between calls, not during one, so there is no way to trip this from the native harness without adding a hook to production code for the test's benefit. The existing latch tests still pin the pre-draw check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
tiny-json.c defined `int errno = 0;` and its header declared it, which is a reserved identifier: on glibc and MinGW `errno` is a macro expanding to a function call, so the definition does not compile there, and anywhere it does, a translation unit that includes <errno.h> gets two different things under one name. It is write-only state -- nothing ever reads it. 7.14.2 and 7.15 both renamed it to json_errno; this line took only half the header change (the extern "C" wrapper) and left the name. This is 7.14.2's version, so the sibling bitcoin-only lines match. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
`git submodule update --remote` is in this repo's documented workflow, and it moves a submodule to the tip of the branch named in .gitmodules. Several keys named a branch that does not contain the commit actually pinned, so running the documented command silently REWINDS a release pin -- for python-keepkey on 7.14.2 and 7.15, and for device-protocol on 7.14.3 and 7.15, where `master` does not carry the release protocol at all. Each key now names a branch verified to contain this tree's pin. Where that branch is a PR branch rather than canonical, the receipt says so and says when to re-pin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
Names the head, tree and CI run this candidate was validated at, itemises the skips rather than summing them, states that every .gitmodules branch key was verified to contain the pin above, and records every audit finding on this line with its resolution in audit-units/ -- including the second pass over the P3s the first pass had triaged rather than re-checked. Nothing but documentation changes here, so the validated head named inside is this commit's parent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
… consent Port of the 7.15 unit (audit/715-dice-only @ bedb6ee) to the bitcoin-only product, which the owner named as the primary target for this feature. dice_input.c/.h, dice.cpp, the capture script, the verifier and both docs are byte-identical to the 7.15 unit. reset.c is identical except where the two products already differed: 7.15 shares the mnemonic pager's scratch with its BIP-85 flow through file-scope buffers, and this product has no BIP-85, so show_mnemonic_pages() owns its scratch as function-local statics here, as the backup pager did before it was factored. Nothing else diverges. What the unit does, on both products: - ResetDevice.dice_entropy alone is MIXED: the device shows its 32-byte draw as 24 BIP-39 words BEFORE the rolls are entered, then seed = SHA256d("KK\x01SM" || draw || SHA256("KK\x01D" || rolls)). - dice_entropy + dice_only is ONLY: seed = SHA256(rolls). Coldcard's Dice-Rolls-Only byte for byte. dice_only without dice_entropy is refused. - The device shows a consent screen naming the mode the host asked for; holding proceeds, the only "no" is cancelling the reset. - The host's EntropyAck is consumed and its bytes dropped in both modes, so the wire flow is unchanged. The default no-dice reset is untouched. - Rolls with any face over 30% are refused before a digest is drawn; the digest is shown in full; tools/verify_dice_seed.py recomputes the wallet offline from the roll string (plus the 24 words for MIXED). Pins, as on the 7.15 unit: deps/device-protocol -> 5517c8de3 (feat/dice-only-field-7.14.3): the dice_only field cherry-picked onto this product's current pin 8545cd5b6. Not canonical up/release-protocol, because the twelve commits between carry EIP-712 streaming fields with no .options here and the pb_callback_t gate would fail. Retires when the product advances its pin. deps/python-keepkey -> c0e9bd8 (feat/dice-modes): host tests for both modes.
Findings from an adversarial review of the unit (86 agents, 3 refuters per finding) and from the first CI runs, all confirmed against the code: - The full 7.15 image failed to link: "Insufficient runtime SRAM: require 16 KiB stack/heap reserve". Its base reserve was 16,420 B against a 16,384 B floor, and the unit had added a 128-byte static for the digest page. That page is now formatted into current_words -- 265 bytes, already CONFIDENTIAL, idle between roll entry and the backup pager -- so the unit adds no static RAM beyond the 4-byte mode. - dice with no_backup is refused with SyntaxError before any screen, as display_random with no_backup was: the modes exist to be checked against the backup words, and a reset that never shows them would put seed material on the OLED under a WARNING that recovery is impossible. - The MIXED consent screen now says the 24 entropy words are NOT a backup, since the pages that follow are otherwise indistinguishable from the backup pages. Both consent bodies are shortened so neither can ever paginate. - Features.supports_dice_modes (28) is reported. Older firmware skips the unknown dice_only field and derives a different wallet without complaint, so hosts and the test suite gate on this bit rather than on a version. - dice_mix() was dead once the modes replaced it; removed with its tests. The non-collision test now compares against the old formula's Python-computed vector instead. A unit test that placed its invalid byte past `count` is corrected. - tools/verify_dice_seed.py looked for a wordlist file the repo does not ship; it now reads the firmware's own bip39_english.c (slicing "abandon" to "zoo", since the file quotes other strings too) and was checked to yield exactly the reference English list. - Stale comments describing an on-device mode selector are corrected; docs/DiceEntropy.md "What happens" now describes the shipped ceremony. Pins: deps/device-protocol -> 0848887cf04a67342c468a40b36fa67d15cdca9b (feat/dice-only-field-7.14.3, adds Features.supports_dice_modes = 28 on the products' dp base); deps/python-keepkey -> 51d7cd112aee21daea44c124310c7b08e921ee07 (feat/dice-modes: capability-gated tests, the digest-subpage and roll-injection driver fixes, no_backup and consent-cancel tests, atlas entries).
feat/dice-modes @ 03c2fe11f704720c7a0230908ae9971717765aa5: the report atlas names the unit's eight Dice gtests in place of the four removed with dice_mix(), which the 7.15 bitcoin-only catalog validation had flagged as missing.
`git submodule update --remote` moves a submodule to the tip of the branch named in .gitmodules. This unit pins device-protocol and python-keepkey to the dice branches, while the keys still named `master` and `reconcile/upstream-sync` -- neither of which contains those commits, so the documented command would silently drop the dice proto field and the dice host tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
Two defects in the offline verification story, which is the entire reason these modes exist. The tool inferred the ceremony from whether --device-words was passed. A user who ran MIXED and forgot the flag got a DICE ONLY derivation -- a perfectly valid, completely different mnemonic -- followed by "the device did not derive the wallet from your rolls. Do not fund it." That is a fraud accusation caused by an omitted argument, and a warning that fires on user error is a warning people learn to ignore. --mode now says it outright, the two impossible combinations are refused with the reason, and the closing text tells the reader to rule out the mode before concluding anything about the device. DiceEntropy.md published the domain tags as `"KK\x01D"` and `"KK\x01SM"`. Written as a C string literal that is wrong: `\x` is greedy, so `\x01D` is one character, 0x1D, and a reader who reimplements the derivation from the published spec gets three bytes where the firmware uses four and can never reproduce an honest device's wallet. The tags are now published as bytes, with the trap named so nobody re-derives it. Checked against the firmware arrays in dice_input.c and the tool constants: 4B 4B 01 44 and 4B 4B 01 53 4D, byte for byte. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V1RPHNqU3JmYFwBYvEKxKn
BitHighlander
force-pushed
the
develop
branch
from
September 15, 2026 19:33
eaf2789 to
fc1e937
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Firmware-only scope correction in progress. The bootloader-coupled durability batch is excluded; its prior integration acceptance is withdrawn. Corrected candidates are staged as fork audit PRs #754 (7.14.2), #755 (7.14.3), and #756 (7.15). Fresh firmware/board and pinned-host suites pass all five variants. Exact-head CI/ARM validation and canonical integration remain pending.
The original power-interruption finding remains open with the coupled remediation deferred under owner scope direction; this is not a clean-release claim. Existing signing and independent firmware hardening fixes are retained. No Copilot review, develop merge, bootloader publication or upstream submission is authorized.
Current scope, exact candidates and evidence.